ENH: Arithmetic with Timestamp-based intervals #36001
Conversation
Updating my fork
Updating my fork
Updation of fork
Updation of fork
… into enh_interv_arith
souris-dev
changed the title
doc/source/whatsnew/v1.2.0.rst
Outdated
| interval - pd.Timestamp("1900-01-01") | ||
|
|
||
| This is valid for addition using `+`, and also when the ends are :class:`Timedelta` s. | ||
| Intervals having ends as Timestamps can also get "added to" or get "subtracted from", |
There was a problem hiding this comment.
Do we need this? Timestamps can't be added/subtracted even if they're not the bounds of an interval
There was a problem hiding this comment.
Timestamps can be subtracted but not added (addition makes no sense). Subtracting Timestamps give Timedeltas.
There was a problem hiding this comment.
thanks @souris-dev for the PR!
Looks good, a few comments in the code
Re: tests generally I think you want to use pytest.mark.parametrize to make the tests easier to read
Re: docs you could add an example for the pandas.Interval API reference
Also, how does this work with Timestamps with different timezone info? Probably no need to mention in the docs but could add tests
| @@ -184,6 +184,184 @@ def test_math_sub(self, closed): | |||
| with pytest.raises(TypeError, match=msg): | |||
| interval - "foo" | |||
|
|
|||
| def test_math_sub_interval_timestamp_timestamp(self, closed): | |||
There was a problem hiding this comment.
I'd name this test_subtraction_interval_mixed_timestamp_timedelta
| result -= Timedelta("3 days 01:00:00") | ||
| assert result == expected | ||
|
|
||
| def test_math_add_interval_timestamp_timestamp(self, closed): |
There was a problem hiding this comment.
test_interval_addition_timestamp
(left and right bounds have to be the same type)
| with pytest.raises(TypeError, match=msg): | ||
| interval += Timestamp("2002-01-08") | ||
|
|
||
| def test_math_sub_interval_timedelta_timestamp(self, closed): |
There was a problem hiding this comment.
test_math_interval_mixed_timedelta_timestamp
| ) | ||
| assert result == expected | ||
|
|
||
| def test_math_add_interval_timestamp_timedelta(self, closed): |
There was a problem hiding this comment.
test_addition_interval_mixed_timestamp_timedelta
| result += Timedelta("1 days 00:00:00") | ||
| assert result == expected | ||
|
|
||
| def test_math_add_interval_timedelta_timedelta(self, closed): |
There was a problem hiding this comment.
test_addition_interval_timedelta
| ) | ||
| assert result == expected | ||
|
|
||
| def test_math_sub_interval_timestamp_timedelta(self, closed): |
There was a problem hiding this comment.
test_subtraction_interval_mixed_timestamp_timedelta
|
@arw2019 - Thanks for the review! |
jreback
added
Interval
| Arithmetic with Timestamp and Timedelta-based Intervals | ||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | ||
|
|
||
| Arithmetic can now be performed on :class:`Interval` s having their left and right |
There was a problem hiding this comment.
is there a reason for all the spaces between backtick and "s"?
| @@ -413,6 +415,8 @@ cdef class Interval(IntervalMixin): | |||
| isinstance(y, numbers.Number) | |||
| or PyDelta_Check(y) | |||
| or is_timedelta64_object(y) | |||
| or isinstance(y, _Timestamp) | |||
| or isinstance(y, _Timedelta) | |||
There was a problem hiding this comment.
the PyDelta_Check call makes the _Timedelta check extraneous. The remaining isinstance checks can be combined.
| @@ -395,6 +395,8 @@ cdef class Interval(IntervalMixin): | |||
| isinstance(y, numbers.Number) | |||
| or PyDelta_Check(y) | |||
| or is_timedelta64_object(y) | |||
| or isinstance(y, _Timestamp) | |||
| or isinstance(y, _Timedelta) | |||
| if the ends were numeric (:issue:`35908`). | ||
|
|
||
| Arithmetic can be performed by directly using arithmetic operators (`-` or `+`), | ||
| so something like this will work: |
There was a problem hiding this comment.
can you add something like this as an example in Interval
|
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this. |
|
closing as stale. if you want to continue, pls ping and can re-open. |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff